Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Make iferr and forr snippets a bit more convenient. #1924

Merged
merged 1 commit into from
Sep 12, 2018
Merged

Make iferr and forr snippets a bit more convenient. #1924

merged 1 commit into from
Sep 12, 2018

Conversation

alecthomas
Copy link
Contributor

@alecthomas alecthomas commented Sep 11, 2018

Fixes #1920.

@@ -62,7 +62,7 @@
},
"for range statement": {
"prefix": "forr",
"body": "for ${1:var} := range ${2:var} {\n\t$0\n}",
"body": "for ${1:_, }${2:var} := range ${3:var} {\n\t$0\n}",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we always need 2 variables, one to hold the index, the other for the element shouldnt the comma be outside the placeholder?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like {1:_},

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rationale for this is that:

  1. I'd say in ~90% of cases you're iterating over slice values and you don't care about the first value, so you just tab through.
  2. When you need to iterate over map keys, or channels, you can just press <backspace> to delete the whole first placeholder. If the comma is outside, you'd need to delete two things - the placeholder and the comma.
  3. For the final case where you want to keep the key, you just type the name and one extra character (,).

That said, if you're unconvinced by my reasoning, I'm happy to change it :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good point

Copy link
Contributor

@ramya-rao-a ramya-rao-a left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @alecthomas, I've left just 1 comment. Please do take a look

@ramya-rao-a ramya-rao-a merged commit 6678d89 into microsoft:master Sep 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants